Skip to content

feat: replace snapshotId with name-based sandbox persistence#369

Open
recoup-coding-agent wants to merge 1 commit intotestfrom
feature/REC-36-sandbox-names
Open

feat: replace snapshotId with name-based sandbox persistence#369
recoup-coding-agent wants to merge 1 commit intotestfrom
feature/REC-36-sandbox-names

Conversation

@recoup-coding-agent
Copy link
Copy Markdown
Collaborator

@recoup-coding-agent recoup-coding-agent commented Mar 29, 2026

Summary

  • Uses Vercel sandbox names feature (accountId as name) instead of snapshotId
  • Sandbox.get({ name }) replaces Sandbox.get({ sandboxId }) throughout
  • Removes snapshotId from coding-agent types, PR state, callbacks, and triggers
  • Removes snapshotId from PATCH /api/sandboxes and GET /api/sandboxes response
  • handleMergeSuccess no longer persists snapshots
  • Updates all related tests (38 files changed)

Part of REC-36. Companion PR: recoupable/tasks#114

Test plan

  • All 1585 API tests pass
  • Verify sandbox creation uses name-based persistence in staging
  • Verify coding agent callback flow works without snapshotId
  • Verify GitHub webhook feedback triggers update-pr without snapshotId

Summary by CodeRabbit

Release Notes

  • Refactor
    • Transitioned sandbox creation and retrieval from snapshot-based to name-based persistence, improving system consistency.
    • Simplified PR state management and feedback handling by removing snapshot ID tracking from thread and PR states.
    • Updated sandbox identification and lookup mechanisms to align with name-based persistence strategy.
    • PR updates now proceed without requiring snapshot information.

Migrates the API to use Vercel's sandbox names feature instead of
snapshot IDs for sandbox identification and state management.

- createSandbox/getActiveSandbox/getSandboxStatus use Sandbox.get({ name })
- processCreateSandbox creates sandboxes with accountId as the name
- Remove snapshotId from coding-agent types, PR state, callbacks, and triggers
- Remove snapshotId from PATCH /api/sandboxes validation and handler
- Remove snapshot_id from GET /api/sandboxes response
- handleMergeSuccess no longer persists snapshots
- Update all related tests (38 files, 107 insertions, 439 deletions)

Co-Authored-By: Paperclip <noreply@paperclip.ing>
@vercel
Copy link
Copy Markdown
Contributor

vercel bot commented Mar 29, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
recoup-api Error Error Mar 29, 2026 0:40am

Request Review

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 29, 2026

📝 Walkthrough

Walkthrough

This PR systematically removes snapshotId tracking from the coding agent PR state management, thread state types, and sandbox creation workflows. It shifts sandbox persistence from snapshot-based identification to name-based strategies using accountId or sandbox name properties, simplifying state lifecycle management across multiple handlers and sandbox operations.

Changes

Cohort / File(s) Summary
Coding Agent Type Definitions
lib/coding-agent/types.ts, lib/coding-agent/prState/types.ts, lib/coding-agent/validateCodingAgentCallback.ts
Removed optional snapshotId?: string field from CodingAgentThreadState interface, CodingAgentPRState interface, and codingAgentCallbackSchema, eliminating snapshot ID from the type system and validation pipeline.
Coding Agent Handlers
lib/coding-agent/handleCodingAgentCallback.ts, lib/coding-agent/handleGitHubWebhook.ts, lib/coding-agent/handlePRCreated.ts, lib/coding-agent/handlers/handleFeedback.ts
Removed snapshotId from thread state and PR state persistence; updated eligibility checks to no longer require snapshotId presence for PR update triggering; adjusted state assignments and task payload construction to omit snapshot references.
PR State Resolution & Merge Handling
lib/coding-agent/resolvePRState.ts, lib/coding-agent/handleMergeSuccess.ts
Removed snapshotId field from returned state shape; eliminated snapshot persistence via upsertAccountSnapshot and related imports/computations in post-merge cleanup.
Task Triggering
lib/trigger/triggerUpdatePR.ts
Removed snapshotId: string property from UpdatePRPayload type; updated JSDoc to describe name-based sandbox resumption without snapshot information.
Sandbox Creation & Retrieval
lib/sandbox/createSandbox.ts, lib/sandbox/createSandboxFromSnapshot.ts, lib/sandbox/getActiveSandbox.ts, lib/sandbox/getOrCreateSandbox.ts, lib/sandbox/getSandboxStatus.ts, lib/sandbox/processCreateSandbox.ts
Switched sandbox API calls from sandboxId parameter to name parameter; changed sandbox ID resolution from sandbox.sandboxId to sandbox.name; shifted creation from snapshot-based (snapshot_id source) to name-based persistence using accountId; removed snapshot lookup/selection dependencies.
Sandbox Metadata Handlers
lib/sandbox/getSandboxesHandler.ts, lib/sandbox/updateSnapshotPatchHandler.ts, lib/sandbox/validateSnapshotPatchBody.ts
Removed snapshot_id extraction/response from handler output; eliminated conditional snapshot metadata update logic; removed snapshotId from request validation schema; updated error messages to reference "sandbox metadata" instead of "snapshot."

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Poem

🚀 Snapshots fade to names more bright,
Sandboxes persist by identity's light,
No more IDs from memory's keep,
Just accountId runs deep,
Cleaner state, simpler flight! ✨

🚥 Pre-merge checks | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Solid & Clean Code ⚠️ Warning Logical contradiction found: getOrCreateSandbox.ts returns fromSnapshot: true for existing sandboxes but false for newly created ones, yet both use identical name-based persistence mechanism. Align fromSnapshot flag semantics across both code paths or refactor to accurately reflect whether sandboxes are created from snapshots in the current architecture.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/REC-36-sandbox-names

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (4)
lib/sandbox/getSandboxesHandler.ts (1)

21-21: ⚠️ Potential issue | 🟡 Minor

JSDoc is out of sync: snapshot_id no longer returned.

The documentation states the response includes snapshot_id, but the implementation at lines 53-59 no longer includes this field.

📝 Suggested fix
- * `@returns` A NextResponse with array of sandbox statuses, plus snapshot_id and github_repo.
+ * `@returns` A NextResponse with array of sandbox statuses, plus github_repo and filetree.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/sandbox/getSandboxesHandler.ts` at line 21, The JSDoc for
getSandboxesHandler is out of sync: it still documents a returned snapshot_id
even though the handler no longer includes that field in its response; update
the JSDoc block for the getSandboxesHandler function to remove snapshot_id (or
otherwise adjust the documented return shape to match the actual returned object
which contains sandbox statuses and github_repo) so the comment accurately
reflects the function's returned fields.
lib/sandbox/updateSnapshotPatchHandler.ts (1)

8-16: ⚠️ Potential issue | 🟡 Minor

Update API route documentation to reflect current implementation.

The PATCH /api/sandboxes documentation at app/api/sandboxes/route.ts claims that snapshotId is a required field in the request body, but the actual implementation validates and processes github_repo instead. Update the endpoint documentation to match the schema validation: the request body accepts an optional github_repo (string URL) and optional account_id (UUID), not snapshotId.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/sandbox/updateSnapshotPatchHandler.ts` around lines 8 - 16, The API docs
for PATCH /api/sandboxes are out of sync with the implementation: update the
documentation in app/api/sandboxes/route.ts (and any JSDoc or comment block in
lib/sandbox/updateSnapshotPatchHandler.ts) to reflect the actual request schema
used by the handler—replace the required snapshotId claim with the correct
fields: optional github_repo (string URL) and optional account_id (UUID); ensure
the docstring, example request body, and any parameter descriptions reference
github_repo and account_id and note they are optional to match the validation
logic in updateSnapshotPatchHandler and the route's schema parsing.
lib/sandbox/processCreateSandbox.ts (1)

26-32: ⚠️ Potential issue | 🟠 Major

Prevent duplicate sandbox persistence rows after name-based migration

Line 26 and Line 29 together now make repeated calls very likely to write the same (account_id, sandbox_id) pair multiple times. That can inflate account_sandboxes and cause duplicate status fetches downstream. Please make persistence idempotent (upsert or insert-if-missing).

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/sandbox/processCreateSandbox.ts` around lines 26 - 32, The
insertAccountSandbox call after createSandbox is currently a blind insert that
can create duplicate (account_id, sandbox_id) rows; change the persistence in
insertAccountSandbox (or wrap the call) to be idempotent by performing an upsert
or insert-if-not-exists based on (account_id, sandbox_id). Locate the insert
logic in insertAccountSandbox and modify it to use ON CONFLICT DO NOTHING /
upsert semantics (or first query for existence and only insert if missing) so
repeated calls to createSandbox/insertAccountSandbox do not create duplicate
rows.
lib/sandbox/getSandboxStatus.ts (1)

12-22: ⚠️ Potential issue | 🟠 Major

Handle legacy persisted sandbox IDs during migration window

Line 12 assumes all stored identifiers are now sandbox names. If any existing account_sandboxes.sandbox_id rows still contain pre-migration IDs, this path will drop them to null via the catch block. Add a compatibility strategy (temporary dual-read or guaranteed backfill before rollout).

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/sandbox/getSandboxStatus.ts` around lines 12 - 22, getSandboxStatus
currently assumes sandboxId is the new "name" and calls Sandbox.get({ name:
sandboxId }) which will throw for legacy IDs and cause null returns; update
getSandboxStatus to first attempt a lookup by name (Sandbox.get({ name:
sandboxId })), and if that returns nothing or throws a not-found error, fall
back to a legacy lookup by id (Sandbox.get({ id: sandboxId })) before returning
null; add a clear deprecation log when the legacy path is used and ensure you
only return null after both attempts fail and after guarding createdAt for
undefined.
🧹 Nitpick comments (4)
lib/sandbox/validateSnapshotPatchBody.ts (1)

13-18: Consider deriving type from Zod schema for consistency.

Per coding guidelines, validation functions should export inferred types. Using z.infer<typeof snapshotPatchBodySchema> would ensure the type stays synchronized with the schema automatically.

♻️ Suggested refactor
-export type SnapshotPatchBody = {
-  /** The account ID to update */
-  accountId: string;
-  /** The GitHub repository URL to associate with the sandbox */
-  githubRepo?: string;
-};
+export type SnapshotPatchBodyRaw = z.infer<typeof snapshotPatchBodySchema>;
+
+export type SnapshotPatchBody = {
+  /** The account ID to update (resolved from auth or body) */
+  accountId: string;
+  /** The GitHub repository URL to associate with the sandbox */
+  githubRepo?: string;
+};
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/sandbox/validateSnapshotPatchBody.ts` around lines 13 - 18, Replace the
manually declared SnapshotPatchBody type with an exported inferred type derived
from the Zod schema: export type SnapshotPatchBody = z.infer<typeof
snapshotPatchBodySchema>; locate the current SnapshotPatchBody declaration and
the snapshotPatchBodySchema in this file (and ensure z is imported from "zod"),
then remove the hand-maintained shape and export the inferred type so the
TypeScript type stays synchronized with snapshotPatchBodySchema.
lib/sandbox/getOrCreateSandbox.ts (1)

21-28: Semantic note: fromSnapshot hardcoded to true for existing sandboxes.

For existing sandboxes, fromSnapshot: true is always returned regardless of whether the sandbox was originally created from a snapshot. If this field is used downstream for logic decisions, consider whether this assumption holds for all cases.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/sandbox/getOrCreateSandbox.ts` around lines 21 - 28, The current
getOrCreateSandbox() always returns fromSnapshot: true for an existing sandbox;
change this to reflect the sandbox's actual origin by reading the sandbox's
stored metadata/flag instead of hardcoding true—inspect the existing object for
a field like existing.metadata.createdFromSnapshot or
existing.createdFromSnapshot (or add such a flag where sandboxes are created),
set fromSnapshot = Boolean(existing.metadata?.createdFromSnapshot ||
existing.createdFromSnapshot) and return that value (fallback to false if the
field is absent) so downstream logic gets the real value.
lib/sandbox/processCreateSandbox.ts (1)

26-32: Extract shared create+persist flow into a sandbox utility

This block duplicates the same pattern now present in lib/sandbox/createSandboxFromSnapshot.ts (create by name, then persist). Consider one shared helper to avoid drift in future sandbox changes.

As per coding guidelines, "Extract shared logic into reusable utilities following Don't Repeat Yourself (DRY) principle".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/sandbox/processCreateSandbox.ts` around lines 26 - 32, The
create-then-persist sandbox flow in processCreateSandbox.ts (calling
createSandbox(...) and then insertAccountSandbox(...)) is duplicated in
createSandboxFromSnapshot; extract this into a shared utility (e.g., a new
helper function like createAndPersistSandbox(accountId) or
createSandboxAndAttachToAccount) that encapsulates calling createSandbox({ name:
... }) and insertAccountSandbox(...), then replace the inline sequence in both
processCreateSandbox and the logic in createSandboxFromSnapshot to call that
helper to avoid duplication and keep sandbox creation/persistence consistent.
lib/sandbox/createSandboxFromSnapshot.ts (1)

5-8: Rename snapshot-oriented API surface to match current behavior

createSandboxFromSnapshot and fromSnapshot now conflict with implementation (Line 30 is always false). Consider renaming the function/result shape to account/name-based terminology and dropping the stale flag.

As per coding guidelines, "KISS: Prefer simple, readable implementations over clever optimizations".

Also applies to: 18-20, 30-30

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/sandbox/createSandboxFromSnapshot.ts` around lines 5 - 8, The API and
return type are snapshot-named but the implementation always sets
fromSnapshot=false; rename the function and result type to reflect account/name
semantics (e.g., createSandboxFromAccount or createSandboxFromName and
CreateSandboxFromAccountResult) and remove the stale fromSnapshot boolean from
the result shape; update all callers and exports of createSandboxFromSnapshot,
CreateSandboxFromSnapshotResult, and any variable named fromSnapshot to use the
new names and rely on the sandbox: Sandbox return only (drop the unused flag) so
the surface matches implementation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@lib/sandbox/getActiveSandbox.ts`:
- Line 22: The call to Sandbox.get uses a { name: ... } parameter which only
exists in Vercel Sandbox SDK v2 beta; either revert to the stable API by passing
{ sandboxId: mostRecent.sandbox_id } (ensure mostRecent.sandbox_id contains the
actual sandbox ID), or if you intend to use v2 beta update package.json to the
v2 beta `@vercel/sandbox` and confirm the database's sandbox_id values are
migrated to user-defined sandbox names; modify the Sandbox.get invocation
accordingly and validate mostRecent.sandbox_id contents before calling.

In `@lib/sandbox/updateSnapshotPatchHandler.ts`:
- Around line 29-32: The upsertAccountSnapshot call in
updateSnapshotPatchHandler only supplies account_id and github_repo, which will
violate DB insert constraints because snapshot_id and expires_at are required;
update the handler to either (A) provide generated defaults when calling
upsertAccountSnapshot (e.g., create a new snapshot_id and compute expires_at) so
inserts include snapshot_id and expires_at, or (B) change the logic to call a
dedicated update function when only updating existing records (avoid upsert),
adjusting the call sites around validated.accountId/validated.githubRepo and
ensuring snapshot_id and expires_at are supplied for insert paths.

---

Outside diff comments:
In `@lib/sandbox/getSandboxesHandler.ts`:
- Line 21: The JSDoc for getSandboxesHandler is out of sync: it still documents
a returned snapshot_id even though the handler no longer includes that field in
its response; update the JSDoc block for the getSandboxesHandler function to
remove snapshot_id (or otherwise adjust the documented return shape to match the
actual returned object which contains sandbox statuses and github_repo) so the
comment accurately reflects the function's returned fields.

In `@lib/sandbox/getSandboxStatus.ts`:
- Around line 12-22: getSandboxStatus currently assumes sandboxId is the new
"name" and calls Sandbox.get({ name: sandboxId }) which will throw for legacy
IDs and cause null returns; update getSandboxStatus to first attempt a lookup by
name (Sandbox.get({ name: sandboxId })), and if that returns nothing or throws a
not-found error, fall back to a legacy lookup by id (Sandbox.get({ id: sandboxId
})) before returning null; add a clear deprecation log when the legacy path is
used and ensure you only return null after both attempts fail and after guarding
createdAt for undefined.

In `@lib/sandbox/processCreateSandbox.ts`:
- Around line 26-32: The insertAccountSandbox call after createSandbox is
currently a blind insert that can create duplicate (account_id, sandbox_id)
rows; change the persistence in insertAccountSandbox (or wrap the call) to be
idempotent by performing an upsert or insert-if-not-exists based on (account_id,
sandbox_id). Locate the insert logic in insertAccountSandbox and modify it to
use ON CONFLICT DO NOTHING / upsert semantics (or first query for existence and
only insert if missing) so repeated calls to createSandbox/insertAccountSandbox
do not create duplicate rows.

In `@lib/sandbox/updateSnapshotPatchHandler.ts`:
- Around line 8-16: The API docs for PATCH /api/sandboxes are out of sync with
the implementation: update the documentation in app/api/sandboxes/route.ts (and
any JSDoc or comment block in lib/sandbox/updateSnapshotPatchHandler.ts) to
reflect the actual request schema used by the handler—replace the required
snapshotId claim with the correct fields: optional github_repo (string URL) and
optional account_id (UUID); ensure the docstring, example request body, and any
parameter descriptions reference github_repo and account_id and note they are
optional to match the validation logic in updateSnapshotPatchHandler and the
route's schema parsing.

---

Nitpick comments:
In `@lib/sandbox/createSandboxFromSnapshot.ts`:
- Around line 5-8: The API and return type are snapshot-named but the
implementation always sets fromSnapshot=false; rename the function and result
type to reflect account/name semantics (e.g., createSandboxFromAccount or
createSandboxFromName and CreateSandboxFromAccountResult) and remove the stale
fromSnapshot boolean from the result shape; update all callers and exports of
createSandboxFromSnapshot, CreateSandboxFromSnapshotResult, and any variable
named fromSnapshot to use the new names and rely on the sandbox: Sandbox return
only (drop the unused flag) so the surface matches implementation.

In `@lib/sandbox/getOrCreateSandbox.ts`:
- Around line 21-28: The current getOrCreateSandbox() always returns
fromSnapshot: true for an existing sandbox; change this to reflect the sandbox's
actual origin by reading the sandbox's stored metadata/flag instead of
hardcoding true—inspect the existing object for a field like
existing.metadata.createdFromSnapshot or existing.createdFromSnapshot (or add
such a flag where sandboxes are created), set fromSnapshot =
Boolean(existing.metadata?.createdFromSnapshot || existing.createdFromSnapshot)
and return that value (fallback to false if the field is absent) so downstream
logic gets the real value.

In `@lib/sandbox/processCreateSandbox.ts`:
- Around line 26-32: The create-then-persist sandbox flow in
processCreateSandbox.ts (calling createSandbox(...) and then
insertAccountSandbox(...)) is duplicated in createSandboxFromSnapshot; extract
this into a shared utility (e.g., a new helper function like
createAndPersistSandbox(accountId) or createSandboxAndAttachToAccount) that
encapsulates calling createSandbox({ name: ... }) and insertAccountSandbox(...),
then replace the inline sequence in both processCreateSandbox and the logic in
createSandboxFromSnapshot to call that helper to avoid duplication and keep
sandbox creation/persistence consistent.

In `@lib/sandbox/validateSnapshotPatchBody.ts`:
- Around line 13-18: Replace the manually declared SnapshotPatchBody type with
an exported inferred type derived from the Zod schema: export type
SnapshotPatchBody = z.infer<typeof snapshotPatchBodySchema>; locate the current
SnapshotPatchBody declaration and the snapshotPatchBodySchema in this file (and
ensure z is imported from "zod"), then remove the hand-maintained shape and
export the inferred type so the TypeScript type stays synchronized with
snapshotPatchBodySchema.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: f20686c2-304f-40c9-b550-3ef8b26e22c7

📥 Commits

Reviewing files that changed from the base of the PR and between 5ce1a75 and 5f49bb4.

⛔ Files ignored due to path filters (19)
  • lib/coding-agent/__tests__/getCodingAgentPRState.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/coding-agent/__tests__/handleCodingAgentCallback.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/coding-agent/__tests__/handleGitHubWebhook.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/coding-agent/__tests__/handleMergeSuccess.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/coding-agent/__tests__/handlePRCreated.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/coding-agent/__tests__/handlers.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/coding-agent/__tests__/onMergeAction.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/coding-agent/__tests__/onSubscribedMessage.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/coding-agent/__tests__/resolvePRState.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/coding-agent/__tests__/setCodingAgentPRState.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/coding-agent/__tests__/validateCodingAgentCallback.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/sandbox/__tests__/createSandbox.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/sandbox/__tests__/createSandboxFromSnapshot.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/sandbox/__tests__/getActiveSandbox.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/sandbox/__tests__/getOrCreateSandbox.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/sandbox/__tests__/getSandboxStatus.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/sandbox/__tests__/getSandboxesHandler.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/sandbox/__tests__/updateSnapshotPatchHandler.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/sandbox/__tests__/validateSnapshotPatchBody.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
📒 Files selected for processing (19)
  • lib/coding-agent/handleCodingAgentCallback.ts
  • lib/coding-agent/handleGitHubWebhook.ts
  • lib/coding-agent/handleMergeSuccess.ts
  • lib/coding-agent/handlePRCreated.ts
  • lib/coding-agent/handlers/handleFeedback.ts
  • lib/coding-agent/prState/types.ts
  • lib/coding-agent/resolvePRState.ts
  • lib/coding-agent/types.ts
  • lib/coding-agent/validateCodingAgentCallback.ts
  • lib/sandbox/createSandbox.ts
  • lib/sandbox/createSandboxFromSnapshot.ts
  • lib/sandbox/getActiveSandbox.ts
  • lib/sandbox/getOrCreateSandbox.ts
  • lib/sandbox/getSandboxStatus.ts
  • lib/sandbox/getSandboxesHandler.ts
  • lib/sandbox/processCreateSandbox.ts
  • lib/sandbox/updateSnapshotPatchHandler.ts
  • lib/sandbox/validateSnapshotPatchBody.ts
  • lib/trigger/triggerUpdatePR.ts
💤 Files with no reviewable changes (5)
  • lib/coding-agent/validateCodingAgentCallback.ts
  • lib/coding-agent/types.ts
  • lib/coding-agent/prState/types.ts
  • lib/coding-agent/handlePRCreated.ts
  • lib/coding-agent/resolvePRState.ts


try {
const sandbox = await Sandbox.get({ sandboxId: mostRecent.sandbox_id });
const sandbox = await Sandbox.get({ name: mostRecent.sandbox_id });
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🌐 Web query:

Vercel Sandbox SDK Sandbox.get method parameters name vs sandboxId

💡 Result:

The Vercel Sandbox SDK's Sandbox.get method takes a sandboxId parameter, which is the unique system-generated identifier (e.g., 'sbx_abc123') of an existing sandbox. This rehydrates an active sandbox instance for resuming work or inspecting logs. It throws an error if the sandbox no longer exists. There is no name parameter for Sandbox.get in the stable SDK; it exclusively uses sandboxId. Official docs confirm: "Sandbox.get({ sandboxId })" with parameter table listing only sandboxId (string, required). Recent beta developments introduce "named sandboxes" (v2), which are persistent and identified by user-defined name (unique within project). In beta SDK, Sandbox.get accepts { name: 'my-sandbox' } instead of sandboxId, resuming from snapshots if stopped. REST API distinguishes: GET /v1/sandboxes/{sandboxId} vs GET /v2/sandboxes/{name}. Use Sandbox.list to find sandboxIds or names. For ephemeral sandboxes (GA), use sandboxId. Check Vercel dashboard or list for IDs.

Citations:


Update to Vercel Sandbox SDK v2 beta or revert Sandbox.get() parameter.

The Vercel Sandbox SDK's stable/GA version accepts only { sandboxId: string } and does not support a name parameter. The change from sandboxId to name will break at runtime unless the project has migrated to the beta v2 SDK.

If moving to beta v2 (which supports named sandboxes), ensure:

  1. The @vercel/sandbox package in package.json targets the beta v2 release
  2. The sandbox_id database column has been fully migrated to store user-defined sandbox names instead of system-generated IDs
  3. All existing sandbox records have been updated, or queries for legacy records will fail

If still using stable SDK, revert to sandboxId parameter and store actual sandbox IDs in the database.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/sandbox/getActiveSandbox.ts` at line 22, The call to Sandbox.get uses a {
name: ... } parameter which only exists in Vercel Sandbox SDK v2 beta; either
revert to the stable API by passing { sandboxId: mostRecent.sandbox_id } (ensure
mostRecent.sandbox_id contains the actual sandbox ID), or if you intend to use
v2 beta update package.json to the v2 beta `@vercel/sandbox` and confirm the
database's sandbox_id values are migrated to user-defined sandbox names; modify
the Sandbox.get invocation accordingly and validate mostRecent.sandbox_id
contents before calling.

Comment on lines 29 to 32
const result = await upsertAccountSnapshot({
account_id: validated.accountId,
...(validated.snapshotId && {
snapshot_id: validated.snapshotId,
expires_at: new Date(Date.now() + SNAPSHOT_EXPIRY_MS).toISOString(),
}),
...(validated.githubRepo && { github_repo: validated.githubRepo }),
github_repo: validated.githubRepo,
});
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Upsert will fail for new accounts due to missing required fields.

This upsertAccountSnapshot call only provides account_id and github_repo. Per the database schema (types/database.types.ts lines 297-328), snapshot_id and expires_at are required for Insert operations. This will throw a PostgreSQL constraint violation for accounts that don't already have a snapshot record.

The test at lib/sandbox/__tests__/updateSnapshotPatchHandler.test.ts (lines 49-75) passes because it mocks the database call, masking this production failure.

🛠️ Potential fix: Generate default values for new records
+import { randomUUID } from "crypto";
+
+const SNAPSHOT_EXPIRY_MS = 7 * 24 * 60 * 60 * 1000; // 7 days
+
 try {
     const result = await upsertAccountSnapshot({
       account_id: validated.accountId,
       github_repo: validated.githubRepo,
+      snapshot_id: randomUUID(),
+      expires_at: new Date(Date.now() + SNAPSHOT_EXPIRY_MS).toISOString(),
     });

Alternatively, if the intent is to only update existing records, consider using a dedicated update function instead of upsert.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/sandbox/updateSnapshotPatchHandler.ts` around lines 29 - 32, The
upsertAccountSnapshot call in updateSnapshotPatchHandler only supplies
account_id and github_repo, which will violate DB insert constraints because
snapshot_id and expires_at are required; update the handler to either (A)
provide generated defaults when calling upsertAccountSnapshot (e.g., create a
new snapshot_id and compute expires_at) so inserts include snapshot_id and
expires_at, or (B) change the logic to call a dedicated update function when
only updating existing records (avoid upsert), adjusting the call sites around
validated.accountId/validated.githubRepo and ensuring snapshot_id and expires_at
are supplied for insert paths.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant